fix(agent): recover tools after stream parse errors#6136
fix(agent): recover tools after stream parse errors#6136usr-bin-roygbiv wants to merge 1 commit into
Conversation
roboomp
added
agent
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P1 — mergeable after a touch. Thanks for the tight, root-cause fix.
What works well:
recoverTransientErrorToolTurn(agent-loop.ts:1625) now extends recovery to explicit transient stream parse errors while correctly gating out refusal/sensitive turns via bothstopDetails.typeand.category, and still requires every retained call to be a known available tool. Combined withretainCompletedToolCallsdropping non-toolcall_endcalls, only fully-parsed, known tools execute — the incomplete-sibling and mixed-known/unknown tests confirm this.isTransientStreamParseError(flags.ts:513) keeps the existingErrorpath untouched (verified:retryable.ts,anthropic.ts:2590,openai-responses.ts:182all passErrorobjects), so the new string branch is additive and only reached by the agent-loop's persistedstopDetails.explanation/errorMessagestrings. No regression to provider retry classification.- Strong branch coverage: parse-recovery, partial-recovery, wrapped-refusal, mixed-known/unknown, content-only, and ordinary-transient cases each assert observable message/tool-execution outcomes, not plumbing. Changelog entry present under
[Unreleased].bun testgreen locally (agent-loop 65 pass, error-aierr 18 pass).
One open item (non-blocking, inline): the string-vs-Error regex asymmetry in flags.ts is deliberate and test-locked but undocumented — the doc comment reads as if both inputs classify identically. A one-line note would prevent a future "looks like a bug" edit.
No correctness or contract concerns; ranking P1 only for that doc gap. Maintainer call on whether to require the comment before merge.
|
|
||
| /** Transient stream corruption where the response was truncated mid-JSON. */ | ||
| export function isTransientStreamParseError(error: unknown): boolean { | ||
| if (typeof error === "string") return STREAM_PARSE_DIAGNOSTIC_PATTERN.test(error); |
There was a problem hiding this comment.
nit: the string branch (STREAM_PARSE_DIAGNOSTIC_PATTERN, narrow — rejects bare truncated/end of file) and the Error branch (STREAM_PARSE_TRUNCATION_PATTERN, broad — matches them) now diverge deliberately. The test at error-aierr.test.ts:148-149 locks this in, and it's the right call (persisted stopDetails.explanation strings are lower-signal than a live Error from the transport), but the doc comment on line 512 still just says "truncated mid-JSON" and gives no hint that the two input types classify differently. A one-line note on why strings get the stricter pattern would save the next reader from assuming the asymmetry is a bug. Non-blocking.
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P1 — mergeable after an optional touch. Thanks for the tight, root-cause fix.
recoverTransientErrorToolTurn(agent-loop.ts:1625) extends recovery to explicit transient stream parse errors, gated so refusal/sensitive turns (checked on bothstopDetails.typeand.category) stay non-executable and every retained call must be a known tool. WithretainCompletedToolCallsdropping any call lacking atoolcall_end, only fully-parsed known tools run.isTransientStreamParseError(flags.ts:513): the new string branch is additive — the three other callers (retryable.ts:64,anthropic.ts:2590,openai-responses.ts:182) passErrorobjects and keep the unchanged broad pattern, so provider retry classification is unaffected.- Branch coverage asserts observable outcomes (recovery, partial-recovery, wrapped-refusal, mixed known/unknown, content-only, ordinary-transient); changelog present; local tests green (agent-loop 65 pass, error-aierr 18 pass).
One non-blocking inline nit: the deliberate narrow-string vs broad-Error regex asymmetry in flags.ts is test-locked but undocumented — a one-line comment would prevent a future "looks like a bug" edit. No correctness or contract concerns.
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P1 — mergeable after an optional touch. Thanks for the tight, root-cause fix.
recoverTransientErrorToolTurn(agent-loop.ts:1625) extends recovery to explicit transient stream parse errors, gated so refusal/sensitive turns (checked on bothstopDetails.typeand.category) stay non-executable and every retained call must be a known tool. WithretainCompletedToolCallsdropping any call lacking atoolcall_end, only fully-parsed known tools run.isTransientStreamParseError(flags.ts:513): the new string branch is additive — the three other callers (retryable.ts:64,anthropic.ts:2590,openai-responses.ts:182) passErrorobjects and keep the unchanged broad pattern, so provider retry classification is unaffected.- Branch coverage asserts observable outcomes; changelog present; local tests green (agent-loop 65 pass, error-aierr 18 pass).
Non-blocking nit: the deliberate narrow-string vs broad-Error regex asymmetry in flags.ts is test-locked but undocumented — a one-line comment would help. No correctness or contract concerns.
2 participants
What
Why
OpenAI Responses can complete a tool call and then emit a malformed non-trailing SSE event. OMP 17.0.5 surfaced
JSON Parse error: Unterminated string, but the agent loop only recovered completed calls forstream_read_error, so the completed tool never executed. The same failure shape was observed independently on macOS and Linux sessions.Testing
bun test packages/agent/test/agent-loop.test.ts packages/agent/test/agent.test.ts packages/ai/test/error-aierr.test.ts packages/ai/test/anthropic-retry.test.ts— 125 passbun check— passbun run ci:test:smoke— passread, malformed middle SSE JSON frame, recovered tool result, continued toOMP_MIDSTREAM_FINAL_LOCAL_OKOMP_MIDSTREAM_FINAL_REMOTE_OKbun checkpasses